-
Notifications
You must be signed in to change notification settings - Fork 187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix repository cache regression #761
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks @souleb
@@ -186,7 +186,6 @@ func (b *remoteChartBuilder) downloadFromRepository(remote *repository.ChartRepo | |||
err = fmt.Errorf("could not load repository index for remote chart reference: %w", err) | |||
return nil, &BuildError{Reason: ErrChartPull, Err: err} | |||
} | |||
defer remote.Unload() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the removal of Unload(), my recommendation in https://github.com/fluxcd/source-controller/pull/690/files#r878152484 becomes more feasible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes 👍 I am taking your recommendation into account in the coming PR that enable dependencies from OCI Helm Repositories.
If implemented this make sure we don't unload an index before caching it during a chart build phase. Signed-off-by: Soule BA <[email protected]>
6581b7a
to
0d4d78f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thanks.
This fixes a regression introduce in #690. We no longer cache Helm repository indexes when cache is enabled because of a defer function in the
remote builder
which unload the index.If implemented this make sure we don't unload an index before caching it during a chart build phase. The index is unloaded downstream after successfully caching it.
Signed-off-by: Soule BA [email protected]